Skip to content

refactor(guest-agent): restrict GetQuote to Intel TDX - #1107

Merged
kvinwang merged 11 commits into
nextfrom
feat/getquote-tdx-only
Aug 24, 2026
Merged

refactor(guest-agent): restrict GetQuote to Intel TDX#1107
kvinwang merged 11 commits into
nextfrom
feat/getquote-tdx-only

Conversation

@kvinwang

Copy link
Copy Markdown
Collaborator

Problem

GetQuote answered on every platform, but only on Intel TDX did it return what its
name promises. Elsewhere — AMD SEV-SNP, AWS Nitro, and friends — it returned an empty
quote and an empty event_log, and put the real evidence in a fifth field,
GetQuoteResponse.attestation, holding a SCALE/msgpack versioned attestation:

message GetQuoteResponse {
  // TDX quote (empty on non-TDX platforms such as AMD SEV-SNP)
  bytes quote = 1;
  ...
  // Platform-adaptive versioned attestation (SCALE/msgpack encoded). Populated
  // on non-TDX platforms; TDX uses quote + event_log above.
  bytes attestation = 5;
}

So a client had to test whether quote was empty to learn which field carried the
answer, and one response type had two mutually exclusive shapes. Attest already
returns exactly that versioned attestation, on every platform, in one shape — the
attestation field was a second, shape-shifting way to ask the same question.

The silent-failure case is worse. Tappd.TdxQuote and RawQuote share this backend
path and copy only quote and event_log into their own response, dropping field 5.
On a non-TDX platform they returned quote: "" with no error — a caller asking for
attestation evidence got an empty string back and nothing to indicate why.

Fix

GetQuote is now Intel TDX only; every other platform gets an error naming Attest
as the replacement. Field 5 is removed and reserved. Tappd.TdxQuote/RawQuote and
Worker.GetAttestationForAppKey share the backend path, so they fail closed there too
instead of returning an empty quote. GCP TDX is unaffected — it carries a real TDX quote.

Four commits, ordered so no commit leaves a regression behind:

  1. fix(gateway): keep the attestation when the quote request fails — stands on its
    own. distributed_certbot.rs treated a get_quote failure as fatal (return Ok(()))
    while tolerating an attest() failure, so a transient QGS outage threw away the
    attestation it could still have had, and saved no record at all. Both are best-effort
    now, and the save is skipped only when both come back empty. Without this, commit 2
    would have silently stopped non-TDX gateways from persisting ACME and cert
    attestations entirely.
  2. refactor(guest-agent): restrict GetQuote to Intel TDX — the RPC change.
  3. refactor(sdk): drop the GetQuote attestation field — Rust, Python, Go, JS.
  4. docs: mark GetQuote as Intel TDX only

Wire compatibility

Field 5 never shipped. It is absent from v0.5.7, v0.5.8 and v0.5.9
(git show v0.5.9:dstack/guest-agent/rpc/proto/agent_rpc.proto), so no released client
can be reading it. It is reserved anyway, following vmm_rpc.proto.

Verification

  • cargo clippy --workspace --all-features -- -D warnings --allow unused_variables clean
    on this branch; cargo fmt --all --check clean.
  • cargo test -p dstack-guest-agent -p dstack-guest-agent-simulator -p dstack-gateway --all-features passes.
  • New regression test simulator_rejects_get_quote_on_non_tdx pins both halves of the
    contract on one attestation: it swaps the TDX fixture's platform evidence for
    PlatformEvidence::SevSnp, asserts simulated_quote_response fails with an error
    containing "Intel TDX only", then asserts simulated_attest_response still succeeds.
  • SDKs: go build ./... and go vet ./dstack/ clean; tsc --noEmit clean;
    cargo check --all-features and cargo fmt --check clean in sdk/rust.
  • Grepped the tree for readers of the removed field — the only remaining .attestation
    hits are on AttestResponse and GpuInfoResponse, which are untouched.

Follow-up (not in this PR)

ct_monitor declares its own QuotedPublicKey with just public_key and quote, and
sends only quote/event_log/vm_config to the verifier — it never reads the
attestation field the gateway publishes, even though /verify accepts it. That CT
monitoring path has therefore never worked on non-TDX, before or after this change.
This PR makes sure the evidence is at least stored and served; teaching ct_monitor
to consume it is separate work.

Copilot AI lite review requested due to automatic review settings August 23, 2026 15:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@kvinwang
kvinwang force-pushed the feat/getquote-tdx-only branch 2 times, most recently from 05664ae to 8d7dbf8 Compare August 23, 2026 17:28
Restricting `GetQuote` to Intel TDX closes the backend path
`Worker.GetAttestationForAppKey` used, so on SEV-SNP and Nitro that
method can no longer answer -- its response type is `GetQuoteResponse`,
and there is no quote to put in it. That leaves the external listener
with no way to attest an app key at all. `Attest` is no substitute: it
lives on the internal unix socket, and an external caller could not use
it even if it did not, because `Attest` takes the report data from the
caller and the caller does not know the app key's public key until the
agent derives it. Deriving the key and building the report data is the
whole reason this method exists.

`AttestAppKey` is that method with a response that can carry every
platform's evidence: same request, `AttestResponse` instead of
`GetQuoteResponse`. Nothing is lost against the old shape -- the
versioned attestation carries the report data and vm_config that
`GetQuoteResponse` exposed as separate fields, and the quote and event
log inside it.

The old method is removed rather than kept alongside. It has shipped
since v0.5.7, so this is a breaking change, but no SDK exposes it, the
tree has no callers outside these tests, and on every platform but TDX
it already returned an empty quote plus the field-5 attestation that
never shipped. Keeping a TDX-only alias of a method that now works
everywhere would cost more than it buys.

`GetAttestationForAppKeyRequest` is renamed to `AttestAppKeyRequest`;
message names do not appear on the wire.

The report-data derivation moves to `app_key_report_data`, which also
gained a bounds check: a public key encoding longer than the 64 bytes of
report data used to panic on the slice copy.
`Attestation::with_report_data` patched the report data into
`PlatformEvidence::Tdx` and `SevSnp` and fell through `other => other`
for everything else -- including `GcpTdx`, whose quote has the identical
TDX layout. The simulator uses this to answer `GetQuote` and `Attest`,
so on a simulated GCP platform it served a quote still bound to the
fixture's report data while echoing the caller's in the response: two
different values, no error, and nothing in the tests looking.

Patch `GcpTdx` the same way. The vTPM quote beside it is left alone; it
commits to PCRs, not to this report data.

Then drop the wildcard that hid this, here and in
`strip_for_config_with_events` above it, which has the same shape and
the same exposure. A wildcard in a transform like these fails open: a
platform nobody thought about gets passed through unmodified and looks
like it was handled. Spelling out Nitro and SEV-SNP costs two lines and
turns the next such omission into a compile error.

`simulator_serves_get_quote_on_gcp_tdx` now asserts the binding rather
than just a non-empty quote, and fails without the fix. It also pins the
other half of the contract this PR defines: GetQuote answers wherever
the platform has a TDX quote, and the vTPM quote GCP's verification also
binds is reachable only through Attest.
@kvinwang
kvinwang force-pushed the feat/getquote-tdx-only branch from 8d7dbf8 to 731aa2d Compare August 23, 2026 17:32
…f production

`with_report_data` has one non-test caller in the tree -- the simulator --
and it is a `pub fn` on a crate that kms, gateway, verifier, guest-agent
and dstack-util all depend on. What it does there is rewrite bytes
568..632 of a signed TDX quote, which leaves the signature covering the
bytes that used to be there. The output is evidence-shaped and cannot
pass verification. That is exactly right for a simulator whose callers
skip verification, and wrong for every other consumer of this crate.

That mismatch is why the missing `GcpTdx` arm went unnoticed for so
long: nothing about the function's placement said "only one caller, and
it is a simulator".

Put it behind a `simulator` feature, `#[cfg(any(test, feature =
"simulator"))]`, passed through `ra-tls` to the one crate that needs it.
A per-package build -- `cargo build --release -p dstack-kms`, the form
the docs prescribe -- now cannot reach it: adding a call to
`kms/src/config.rs` fails with `no method named with_report_data`.
`cargo build --workspace` unifies features and does still compile it in,
so this is a statement of intent with teeth on the release path, not a
hard wall.

Adding the gate immediately found the one legitimate cfg(test) user
outside the simulator, `guest-agent`'s mock platform, which now declares
the feature in `[dev-dependencies]` -- so guest-agent's production build
drops it too.

Also splits out `with_stack_report_data`. The simulator's seeded path
called the full version and then overwrote the quote with a freshly
generated, properly signed one, so the quote-patching half was dead work
that read as if it mattered. It now asks for the half it actually uses.
… RPC

The first commit in this series stopped `generate_and_save_attestation`
from discarding the attestation when the quote request fails, so a
non-TDX node -- or a TDX node during a QGS outage -- now saves a record
carrying only `attestation`. Nothing could read it: `CertAttestationInfo`
has fields for the public key and the quote and nothing else.

That made the transient-failure case worse than before, not better.
`save_cert_attestation` overwrites the `latest` pointer unconditionally,
so one failed quote request on a TDX node replaced a record the admin
RPC could read with one it could not.

Add `attestation` as field 5, additive, and map it in both the `latest`
and `history` views. Documents on `quote` when it is empty and what
carries the evidence instead.

ct_monitor still bails on an empty quote, so it remains TDX-only; that
is the follow-up this PR's description names.
The gate is `Attestation::get_tdx_quote_bytes()`, which is `Some` for
`DstackGcpTdx` as well as `DstackTdx`, so GCP Confidential VMs are
served. The docs said they get an error, which was wrong -- and wrong in
the direction that matters: a relying party told "GCP errors here" and
then handed a GCP quote has no reason to look for the vTPM quote GCP's
verification binds alongside it. `GetQuoteResponse` has no field for
that quote, and the raw-quote path in the verifier rebuilds its input as
`AttestationQuote::DstackTdx`, so it checks the TDX half and stops.

Say what actually happens instead: GetQuote needs TDX, GCP gets the TDX
half only, and Attest is the answer both where GetQuote errors and where
it under-reports. Updated in the proto, docs/usage.md, sdk/curl/api.md,
the verifier README, and all four SDKs.

Also fixes the `[Attest](#8-attest)` anchor in `sdk/curl/api.md` and
`dstack/verifier/README.md` -- Attest is section 7. The verifier README
has pointed at the wrong section since Sign and Verify were inserted
above it.
@kvinwang
kvinwang force-pushed the feat/getquote-tdx-only branch from 731aa2d to 4d42fa7 Compare August 24, 2026 01:02
`tdx_event_log()` reaches `Tdx | GcpTdx`; `tdx_event_log_mut()` reached
only `Tdx`. The two drifted apart because both ended in `_ => None`, so
adding `GcpTdx` to one and not the other compiled fine. The simulator
fills V2 digest preimages through the mut accessor, so on a GCP
attestation it silently filled nothing and returned events without the
preimage `agent_rpc.proto` tells clients to verify against the digest.

Drop the wildcard from all eight accessors on `PlatformEvidence` and
spell the variants out. This is the same failure `with_report_data` had
two commits ago; a wildcard is how a family of per-platform methods
disagrees with itself without anyone noticing.

`tdx_event_log_accessors_agree_on_gcp_tdx` pins the read and mut halves
to the same set and fails against the old match.

Also corrects the comment on the `GcpTdx` arm of `with_report_data`. It
claimed the vTPM quote was left alone because it commits to PCRs rather
than to the report data. That is wrong: its `qualified_data` is
`sha256(tdx_quote)` (attestation.rs:1087), so patching the quote severs
that binding too. The behaviour is right -- re-deriving it would need
the AK to re-sign, and the patch has already invalidated the quote
signature -- but the reason given was not.
The algorithm reached `get_key` verbatim, and `get_key` knows only
`ed25519` and `secp256k1`, so `AttestAppKey{algorithm:
"secp256k1_prehashed"}` came back "Unsupported algorithm" -- while
`Sign` accepts that name and has mapped it to its base key since it was
added. A client that signs prehashed had no way to attest the key it
signs with, under the name it signs with.

Prehashing is a signing mode, not a key type: the same secp256k1 key
signs both ways and has the same public key, so derive it under the base
name exactly as `Sign` does. The arm for it in the match below was
already there and simply unreachable; the PR's own prehashed sign test
sidestepped this by asking for "secp256k1".

`test_attest_app_key_accepts_secp256k1_prehashed` asserts both names
attest the same public key.
@kvinwang
kvinwang merged commit 0182d85 into next Aug 24, 2026
16 checks passed
@kvinwang
kvinwang deleted the feat/getquote-tdx-only branch August 24, 2026 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants